From 80a81334f6d6799d2db00cd74f9bc88de3e0ae7c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 24 Jun 2014 22:06:11 -0700 Subject: [PATCH] Fix lots of windows tests * Add a convenience method bin() for generating the name of a binary. On windows this remembers to append `.exe`. * Stop executing relative paths to binaries and relying on PATH. This is suffering from rust-lang/rust#15149 and failing to spawn processes on windows. Additionally, this allows the tests to work with a pre-installed cargo becuase the freshly built executables are precisely specified. * A new function, escape_path(), was added for tests. When generated source files with paths, this function needs to be called to properly escape the \-character that appears in windows path names. Without this function we would be generating invalid TOML and rust. --- src/cargo/core/package_id.rs | 14 ++-- tests/support/mod.rs | 14 +++- tests/test_cargo_compile.rs | 102 ++++++++++++++------------ tests/test_cargo_compile_git_deps.rs | 47 ++++++------ tests/test_cargo_compile_path_deps.rs | 29 ++++---- 5 files changed, 112 insertions(+), 94 deletions(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index f777c0756..719c709a5 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -10,7 +10,7 @@ use serialize::{ Decoder }; -use util::{CargoResult, CargoError, FromError}; +use util::{CargoResult, CargoError}; use core::source::Location; trait ToVersion { @@ -125,7 +125,7 @@ impl>> Decodable> for PackageId { PackageId::new( vector.get(0).as_slice(), vector.get(1).as_slice(), - &Location::parse(vector.get(2).as_slice()).unwrap()) + &try!(Location::parse(vector.get(2).as_slice()))) } } @@ -139,12 +139,14 @@ impl> Encodable for PackageId { #[cfg(test)] mod tests { use super::{PackageId, central_repo}; + use core::source::Location; #[test] fn invalid_version_handled_nicely() { - assert!(PackageId::new("foo", "1.0", central_repo).is_err()); - assert!(PackageId::new("foo", "1", central_repo).is_err()); - assert!(PackageId::new("foo", "bar", central_repo).is_err()); - assert!(PackageId::new("foo", "", central_repo).is_err()); + let repo = Location::parse(central_repo).unwrap(); + assert!(PackageId::new("foo", "1.0", &repo).is_err()); + assert!(PackageId::new("foo", "1", &repo).is_err()); + assert!(PackageId::new("foo", "bar", &repo).is_err()); + assert!(PackageId::new("foo", "", &repo).is_err()); } } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 8cac9db37..b8bc51142 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -66,10 +66,14 @@ impl ProjectBuilder { } pub fn root(&self) -> Path { - self.root.clone() + self.root.clone() } - pub fn process(&self, program: &str) -> ProcessBuilder { + pub fn bin(&self, b: &str) -> Path { + self.root.join("target").join(format!("{}{}", b, os::consts::EXE_SUFFIX)) + } + + pub fn process(&self, program: T) -> ProcessBuilder { process(program) .cwd(self.root()) .env("HOME", Some(paths::home().display().to_str().as_slice())) @@ -78,7 +82,7 @@ impl ProjectBuilder { pub fn cargo_process(&self, program: &str) -> ProcessBuilder { self.build(); - self.process(program) + self.process(cargo_dir().join(program)) } pub fn file(mut self, path: B, @@ -347,3 +351,7 @@ impl Tap for T { self } } + +pub fn escape_path(p: &Path) -> String { + p.display().to_str().as_slice().replace("\\", "\\\\") +} diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 6354ad422..91cb04807 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -1,10 +1,11 @@ use std::io::fs; use std::os; +use std::path; -use support::{ResultTest,project,execs,main_file}; -use hamcrest::{assert_that,existing_file}; +use support::{ResultTest, project, execs, main_file, escape_path}; +use hamcrest::{assert_that, existing_file}; use cargo; -use cargo::util::{process,realpath}; +use cargo::util::{process, realpath}; fn setup() { } @@ -31,12 +32,10 @@ test!(cargo_compile_simple { .file("src/foo.rs", main_file(r#""i am foo""#, []).as_slice()); assert_that(p.cargo_process("cargo-build"), execs()); - assert_that(&p.root().join("target/foo"), existing_file()); - - let target = p.root().join("target"); + assert_that(&p.bin("foo"), existing_file()); assert_that( - process("foo").extra_path(target), + process(p.bin("foo")), execs().with_stdout("i am foo\n")); }) @@ -104,14 +103,15 @@ test!(cargo_compile_with_invalid_code { execs() .with_status(101) .with_stderr(format!("\ -src/foo.rs:1:1: 1:8 error: expected item but found `invalid` -src/foo.rs:1 invalid rust code! +{filename}:1:1: 1:8 error: expected item but found `invalid` +{filename}:1 invalid rust code! ^~~~~~~ Could not execute process \ -`rustc src/foo.rs --crate-type bin --out-dir {} -L {} -L {}` (status=101)\n", +`rustc {filename} --crate-type bin --out-dir {} -L {} -L {}` (status=101)\n", target.display(), target.display(), - target.join("deps").display()).as_slice())); + target.join("deps").display(), + filename = format!("src{}foo.rs", path::SEP)).as_slice())); }) test!(cargo_compile_with_warnings_in_the_root_package { @@ -121,12 +121,12 @@ test!(cargo_compile_with_warnings_in_the_root_package { assert_that(p.cargo_process("cargo-build"), execs() - .with_stderr("\ -src/foo.rs:1:14: 1:26 warning: code is never used: `dead`, #[warn(dead_code)] \ + .with_stderr(format!("\ +{filename}:1:14: 1:26 warning: code is never used: `dead`, #[warn(dead_code)] \ on by default -src/foo.rs:1 fn main() {} fn dead() {} +{filename}:1 fn main() {{}} fn dead() {{}} ^~~~~~~~~~~~ -")); +", filename = format!("src{}foo.rs", path::SEP).as_slice()))); }) test!(cargo_compile_with_warnings_in_a_dep_package { @@ -136,7 +136,7 @@ test!(cargo_compile_with_warnings_in_a_dep_package { p = p .file(".cargo/config", format!(r#" paths = ["{}"] - "#, bar.display()).as_slice()) + "#, escape_path(&bar)).as_slice()) .file("Cargo.toml", r#" [project] @@ -184,10 +184,10 @@ test!(cargo_compile_with_warnings_in_a_dep_package { COMPILING, main.display())) .with_stderr("")); - assert_that(&p.root().join("target/foo"), existing_file()); + assert_that(&p.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(p.root().join("target")), + cargo::util::process(p.bin("foo")), execs().with_stdout("test passed\n")); }) @@ -199,7 +199,7 @@ test!(cargo_compile_with_nested_deps_shorthand { p = p .file(".cargo/config", format!(r#" paths = ["{}", "{}"] - "#, bar.display(), baz.display()).as_slice()) + "#, escape_path(&bar), escape_path(&baz)).as_slice()) .file("Cargo.toml", r#" [project] @@ -260,10 +260,10 @@ test!(cargo_compile_with_nested_deps_shorthand { .exec_with_output() .assert(); - assert_that(&p.root().join("target/foo"), existing_file()); + assert_that(&p.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(p.root().join("target")), + cargo::util::process(p.bin("foo")), execs().with_stdout("test passed\n")); }) @@ -275,7 +275,7 @@ test!(cargo_compile_with_nested_deps_longhand { p = p .file(".cargo/config", format!(r#" paths = ["{}", "{}"] - "#, bar.display(), baz.display()).as_slice()) + "#, escape_path(&bar), escape_path(&baz)).as_slice()) .file("Cargo.toml", r#" [project] @@ -334,10 +334,10 @@ test!(cargo_compile_with_nested_deps_longhand { assert_that(p.cargo_process("cargo-build"), execs()); - assert_that(&p.root().join("target/foo"), existing_file()); + assert_that(&p.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(p.root().join("target")), + cargo::util::process(p.bin("foo")), execs().with_stdout("test passed\n")); }) @@ -373,7 +373,7 @@ test!(custom_build { build = "{}" [[bin]] name = "foo" - "#, build.root().join("target/foo").display())) + "#, escape_path(&build.bin("foo")))) .file("src/foo.rs", r#" fn main() {} "#); @@ -394,7 +394,8 @@ test!(custom_build_failure { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] name = "foo" + [[bin]] + name = "foo" "#) .file("src/foo.rs", r#" fn main() { fail!("nope") } @@ -412,18 +413,19 @@ test!(custom_build_failure { authors = ["wycats@example.com"] build = "{}" - [[bin]] name = "foo" - "#, build.root().join("target/foo").display())) + [[bin]] + name = "foo" + "#, escape_path(&build.bin("foo")))) .file("src/foo.rs", r#" fn main() {} "#); assert_that(p.cargo_process("cargo-build"), execs().with_status(101).with_stderr(format!("\ -Could not execute process `{}` (status=101) ---- stderr -task '
' failed at 'nope', src/foo.rs:2 - -", build.root().join("target/foo").display()))); +Could not execute process `{}` (status=101)\n\ +--- stderr\n\ +task '
' failed at 'nope', {filename}:2\n\ +\n\ +", build.bin("foo").display(), filename = format!("src{}foo.rs", path::SEP)))); }) test!(custom_build_env_vars { @@ -437,7 +439,8 @@ test!(custom_build_env_vars { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] name = "foo" + [[bin]] + name = "foo" "#) .file("src/foo.rs", format!(r#" use std::os; @@ -446,8 +449,8 @@ test!(custom_build_env_vars { assert_eq!(os::getenv("DEPS_DIR").unwrap(), "{}".to_str()); }} "#, - p.root().join("target").display(), - p.root().join("target/deps").display())); + escape_path(&p.root().join("target")), + escape_path(&p.root().join("target").join("deps")))); assert_that(build.cargo_process("cargo-build"), execs().with_status(0)); @@ -460,8 +463,9 @@ test!(custom_build_env_vars { authors = ["wycats@example.com"] build = "{}" - [[bin]] name = "foo" - "#, build.root().join("target/foo").display())) + [[bin]] + name = "foo" + "#, escape_path(&build.bin("foo")))) .file("src/foo.rs", r#" fn main() {} "#); @@ -480,7 +484,8 @@ test!(custom_build_in_dependency { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] name = "foo" + [[bin]] + name = "foo" "#) .file("src/foo.rs", format!(r#" use std::os; @@ -489,15 +494,15 @@ test!(custom_build_in_dependency { assert_eq!(os::getenv("DEPS_DIR").unwrap(), "{}".to_str()); }} "#, - p.root().join("target/deps").display(), - p.root().join("target/deps").display())); + escape_path(&p.root().join("target/deps")), + escape_path(&p.root().join("target/deps")))); assert_that(build.cargo_process("cargo-build"), execs().with_status(0)); p = p .file(".cargo/config", format!(r#" paths = ["{}"] - "#, bar.display()).as_slice()) + "#, escape_path(&bar)).as_slice()) .file("Cargo.toml", r#" [project] @@ -505,8 +510,10 @@ test!(custom_build_in_dependency { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] name = "foo" - [dependencies] bar = "0.5.0" + [[bin]] + name = "foo" + [dependencies] + bar = "0.5.0" "#) .file("src/foo.rs", r#" extern crate bar; @@ -520,8 +527,9 @@ test!(custom_build_in_dependency { authors = ["wycats@example.com"] build = "{}" - [[lib]] name = "bar" - "#, build.root().join("target/foo").display())) + [[lib]] + name = "bar" + "#, escape_path(&build.bin("foo")))) .file("bar/src/bar.rs", r#" pub fn bar() {} "#); @@ -554,7 +562,7 @@ test!(many_crate_types { let mut files: Vec = files.iter().filter_map(|f| { match f.filename_str().unwrap() { "deps" => None, - s if !s.starts_with("lib") => None, + s if s.contains("fingerprint") => None, s => Some(s.to_str()) } }).collect(); diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index 9c50a90c2..052bc7e6e 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -1,6 +1,7 @@ use std::io::File; use support::{ProjectBuilder, ResultTest, project, execs, main_file, paths}; +use support::{escape_path, cargo_dir}; use hamcrest::{assert_that,existing_file}; use cargo; use cargo::util::{ProcessError, process}; @@ -73,18 +74,18 @@ test!(cargo_compile_simple_git_dep { [dependencies.dep1] - git = "file://{}" + git = "file:{}" [[bin]] name = "foo" - "#, git_project.root().display())) + "#, escape_path(&git_project.root()))) .file("src/foo.rs", main_file(r#""{}", dep1::hello()"#, ["dep1"])); let root = project.root(); let git_root = git_project.root(); - assert_that(project.cargo_process("cargo-build"), + assert_that(project.cargo_process("cargo-build").arg("--verbose"), execs() .with_stdout(format!("{} git repository `file:{}`\n\ {} dep1 v0.5.0 (file:{})\n\ @@ -94,10 +95,10 @@ test!(cargo_compile_simple_git_dep { COMPILING, root.display())) .with_stderr("")); - assert_that(&project.root().join("target/foo"), existing_file()); + assert_that(&project.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(project.root().join("target")), + cargo::util::process(project.bin("foo")), execs().with_stdout("hello world\n")); }) @@ -136,13 +137,13 @@ test!(cargo_compile_git_dep_branch { [dependencies.dep1] - git = "file://{}" + git = "file:{}" branch = "branchy" [[bin]] name = "foo" - "#, git_project.root().display())) + "#, escape_path(&git_project.root()))) .file("src/foo.rs", main_file(r#""{}", dep1::hello()"#, ["dep1"])); let root = project.root(); @@ -158,10 +159,10 @@ test!(cargo_compile_git_dep_branch { COMPILING, root.display())) .with_stderr("")); - assert_that(&project.root().join("target/foo"), existing_file()); + assert_that(&project.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(project.root().join("target")), + cargo::util::process(project.bin("foo")), execs().with_stdout("hello world\n")); }) @@ -201,13 +202,13 @@ test!(cargo_compile_git_dep_tag { [dependencies.dep1] - git = "file://{}" + git = "file:{}" tag = "v0.1.0" [[bin]] name = "foo" - "#, git_project.root().display())) + "#, escape_path(&git_project.root()))) .file("src/foo.rs", main_file(r#""{}", dep1::hello()"#, ["dep1"])); let root = project.root(); @@ -223,10 +224,10 @@ test!(cargo_compile_git_dep_tag { COMPILING, root.display())) .with_stderr("")); - assert_that(&project.root().join("target/foo"), existing_file()); + assert_that(&project.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(project.root().join("target")), + cargo::util::process(project.bin("foo")), execs().with_stdout("hello world\n")); }) test!(cargo_compile_with_nested_paths { @@ -284,12 +285,12 @@ test!(cargo_compile_with_nested_paths { [dependencies.dep1] version = "0.5.0" - git = "file://{}" + git = "file:{}" [[bin]] name = "parent" - "#, git_project.root().display())) + "#, escape_path(&git_project.root()))) .file("src/parent.rs", main_file(r#""{}", dep1::hello()"#, ["dep1"]).as_slice()); @@ -297,10 +298,10 @@ test!(cargo_compile_with_nested_paths { .exec_with_output() .assert(); - assert_that(&p.root().join("target/parent"), existing_file()); + assert_that(&p.bin("parent"), existing_file()); assert_that( - cargo::util::process("parent").extra_path(p.root().join("target")), + cargo::util::process(p.bin("parent")), execs().with_stdout("hello world\n")); }) @@ -333,12 +334,12 @@ test!(recompilation { [dependencies.bar] version = "0.5.0" - git = "file://{}" + git = "file:{}" [[bin]] name = "foo" - "#, git_project.root().display())) + "#, escape_path(&git_project.root()))) .file("src/foo.rs", main_file(r#""{}", bar::bar()"#, ["bar"]).as_slice()); @@ -352,7 +353,7 @@ test!(recompilation { COMPILING, p.root().display()))); // Don't recompile the second time - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", FRESH, git_project.root().display(), @@ -363,13 +364,13 @@ test!(recompilation { pub fn bar() { println!("hello!"); } "#).assert(); - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", FRESH, git_project.root().display(), FRESH, p.root().display()))); - assert_that(p.process("cargo-build").arg("-u"), + assert_that(p.process(cargo_dir().join("cargo-build")).arg("-u"), execs().with_stdout(format!("{} git repository `file:{}`\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", @@ -385,7 +386,7 @@ test!(recompilation { git_project.process("git").args(["commit", "-m", "test"]).exec_with_output() .assert(); - assert_that(p.process("cargo-build").arg("-u"), + assert_that(p.process(cargo_dir().join("cargo-build")).arg("-u"), execs().with_stdout(format!("{} git repository `file:{}`\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 412c81748..f3063d678 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -1,8 +1,8 @@ use std::io::File; use std::io::timer; -use support::{ResultTest,project,execs,main_file}; -use hamcrest::{assert_that,existing_file}; +use support::{ResultTest, project, execs, main_file, escape_path, cargo_dir}; +use hamcrest::{assert_that, existing_file}; use cargo; use cargo::util::{process}; @@ -76,10 +76,10 @@ test!(cargo_compile_with_nested_deps_shorthand { .exec_with_output() .assert(); - assert_that(&p.root().join("target/foo"), existing_file()); + assert_that(&p.bin("foo"), existing_file()); assert_that( - cargo::util::process("foo").extra_path(p.root().join("target")), + cargo::util::process(p.bin("foo")), execs().with_stdout("test passed\n")); }) @@ -89,7 +89,7 @@ test!(no_rebuild_dependency { p = p .file(".cargo/config", format!(r#" paths = ["{}"] - "#, bar.display()).as_slice()) + "#, escape_path(&bar)).as_slice()) .file("Cargo.toml", r#" [project] @@ -123,14 +123,14 @@ test!(no_rebuild_dependency { COMPILING, bar.display(), COMPILING, p.root().display()))); // This time we shouldn't compile bar - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", FRESH, bar.display(), FRESH, p.root().display()))); p.build(); // rebuild the files (rewriting them in the process) - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", COMPILING, bar.display(), @@ -144,7 +144,7 @@ test!(deep_dependencies_trigger_rebuild { p = p .file(".cargo/config", format!(r#" paths = ["{}", "{}"] - "#, bar.display(), baz.display()).as_slice()) + "#, escape_path(&bar), escape_path(&baz)).as_slice()) .file("Cargo.toml", r#" [project] @@ -197,7 +197,7 @@ test!(deep_dependencies_trigger_rebuild { COMPILING, baz.display(), COMPILING, bar.display(), COMPILING, p.root().display()))); - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} baz v0.5.0 (file:{})\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", @@ -213,7 +213,7 @@ test!(deep_dependencies_trigger_rebuild { File::create(&p.root().join("baz/src/baz.rs")).write_str(r#" pub fn baz() { println!("hello!"); } "#).assert(); - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} baz v0.5.0 (file:{})\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", @@ -226,12 +226,11 @@ test!(deep_dependencies_trigger_rebuild { extern crate baz; pub fn bar() { println!("hello!"); baz::baz(); } "#).assert(); - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} baz v0.5.0 (file:{})\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", - FRESH, - baz.display(), + FRESH, baz.display(), COMPILING, bar.display(), COMPILING, p.root().display()))); }) @@ -243,7 +242,7 @@ test!(no_rebuild_two_deps { p = p .file(".cargo/config", format!(r#" paths = ["{}", "{}"] - "#, bar.display(), baz.display()).as_slice()) + "#, escape_path(&bar), escape_path(&baz)).as_slice()) .file("Cargo.toml", r#" [project] @@ -296,7 +295,7 @@ test!(no_rebuild_two_deps { COMPILING, baz.display(), COMPILING, bar.display(), COMPILING, p.root().display()))); - assert_that(p.process("cargo-build"), + assert_that(p.process(cargo_dir().join("cargo-build")), execs().with_stdout(format!("{} baz v0.5.0 (file:{})\n\ {} bar v0.5.0 (file:{})\n\ {} foo v0.5.0 (file:{})\n", -- 2.30.2